Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces a frontend prototype for a queue management system using React 19, TypeScript, Vite 7, and Tailwind CSS 4. The application allows users to join a virtual queue, view their position, and see other people waiting in line.
Changes:
- Set up a modern React + TypeScript project with Vite as the build tool
- Implemented queue management functionality with a custom React hook
- Created responsive UI components for queue visualization and user interaction
Reviewed changes
Copilot reviewed 23 out of 26 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Defines project dependencies including React 19.2, Vite 7.2, and Tailwind CSS 4.1 |
| vite.config.ts | Configures Vite with React plugin support |
| tsconfig.*.json | TypeScript configuration files for app and node environments |
| tailwind.config.js | Tailwind CSS configuration with content paths |
| src/hooks/useQueue.js | Custom hook for queue state management (should be TypeScript) |
| src/components/*.tsx | React components for queue UI including header, body, preview, and forms |
| src/main.tsx | Application entry point with React root rendering |
| src/index.css | Global styles with Tailwind imports |
| index.html | HTML entry point for the SPA |
| eslint.config.js | ESLint configuration for TypeScript and React |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ]); | ||
|
|
||
| const [name, setName] = useState(''); |
There was a problem hiding this comment.
The state variable 'name' is declared but never used in this hook. It should be removed or exported if needed.
|
|
||
| const addToQueue = (nameFromForm) => { | ||
| const newPerson = { | ||
| id: Date.now(), |
There was a problem hiding this comment.
Using Date.now() for generating IDs can lead to collisions if multiple users join in quick succession. Consider using a more robust ID generation method like crypto.randomUUID() or a counter-based approach.
| @@ -0,0 +1,28 @@ | |||
| // src/components/QueuePosition.jsx | |||
|
|
|||
| export function QueuePosition({ currentUser, position, peopleAhead, onLeave }) { | |||
There was a problem hiding this comment.
Props are not typed. Consider adding TypeScript prop types for type safety.
| export function QueuePosition({ currentUser, position, peopleAhead, onLeave }) { | |
| export interface QueuePositionProps { | |
| currentUser: { | |
| name: string; | |
| }; | |
| position: number; | |
| peopleAhead: number; | |
| onLeave: () => void; | |
| } | |
| export function QueuePosition({ currentUser, position, peopleAhead, onLeave }: QueuePositionProps) { |
| @@ -0,0 +1,11 @@ | |||
| // src/components/QueueNotification.jsx | |||
|
|
|||
| export function QueueNotification({ peopleAhead }) { | |||
There was a problem hiding this comment.
Props are not typed. Consider adding TypeScript prop types for type safety.
| export function QueueNotification({ peopleAhead }) { | |
| interface QueueNotificationProps { | |
| peopleAhead: number; | |
| } | |
| export function QueueNotification({ peopleAhead }: QueueNotificationProps) { |
| export function QueueBody({ | ||
| currentUser, | ||
| position, | ||
| peopleAhead, | ||
| queue, | ||
| onJoin, | ||
| onLeave |
There was a problem hiding this comment.
Props are not typed. Consider adding TypeScript prop types for type safety.
| @@ -0,0 +1,54 @@ | |||
| import { useState } from 'react'; | |||
There was a problem hiding this comment.
This file should use TypeScript (.tsx extension) to match the project configuration. The project uses TypeScript throughout, as indicated by tsconfig files, but this hook is defined as a JavaScript file.
| }; | ||
| setQueue([...queue, newPerson]); | ||
| setCurrentUser(newPerson); | ||
| setName(''); |
There was a problem hiding this comment.
The setName('') call has no effect since 'name' state is not being used anywhere in this hook. This line should be removed.
| @@ -0,0 +1,70 @@ | |||
| export function QueuePreview({ queue, currentUser, onLeave }) { | |||
There was a problem hiding this comment.
Props are not typed. Consider adding TypeScript prop types for type safety.
| export function QueuePreview({ queue, currentUser, onLeave }) { | |
| type QueuePerson = { | |
| id: string | number; | |
| name: string; | |
| }; | |
| interface QueuePreviewProps { | |
| queue: QueuePerson[]; | |
| currentUser?: { id: string | number; name: string }; | |
| onLeave: () => void; | |
| } | |
| export function QueuePreview({ queue, currentUser, onLeave }: QueuePreviewProps) { |
|
|
||
| import { useState } from 'react'; | ||
|
|
||
| export function QueueJoinForm({ onJoin }) { |
There was a problem hiding this comment.
Props are not typed. Consider adding TypeScript prop types for type safety.
| export function QueueJoinForm({ onJoin }) { | |
| interface QueueJoinFormProps { | |
| onJoin: (name: string) => void; | |
| } | |
| export function QueueJoinForm({ onJoin }: QueueJoinFormProps) { |
andrewsoonqn
left a comment
There was a problem hiding this comment.
Looks good for a prototype. Thanks for the good work!
Some tweaks:
- can u rename the folder to just frontend/ ?
- GH copilot left some good comments, can u make changes accordingly
| return ( | ||
| <div className="bg-[#1e2936] p-6"> | ||
| <h1 className="text-2xl font-bold text-white text-center"> | ||
| NUSC Queuebot SOSD |
yihao03
left a comment
There was a problem hiding this comment.
At its current stage, the UI seems to be optimised for mobile view which is fine, assuming that most users would likely access this app on their phone. However, please also implement responsive design for bigger screens to fully utilize the screen real estate.
There could also be privacy concerns of showing everyone the full name of people in the queue. I am not sure how relevant that is.
| export function QueueHeader() { | ||
| return ( | ||
| <div className="bg-[#1e2936] p-6"> | ||
| <h1 className="text-2xl font-bold text-white text-center"> | ||
| NUSC Queuebot SOSD | ||
| </h1> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
avoid premature optimisation/abstraction. same applies to the footer and other components that are only used once
| const [queue, setQueue] = useState([ | ||
| { id: 1, name: "Alice" }, | ||
| { id: 2, name: "Bob" }, | ||
| { id: 3, name: "Charlie" }, | ||
| { id: 4, name: "Dave" }, | ||
| { id: 5, name: "Ellie" }, | ||
| { id: 6, name: "Florence" }, | ||
| { id: 7, name: "Gavin" }, | ||
| { id: 8, name: "Hendrick" }, | ||
|
|
||
|
|
||
| ]); |
There was a problem hiding this comment.
understand that this is a mockup data for the queue. however try to be consistent and use typescript over all the files. A proper type should be defined for members in the queue too. Consider including other useful fields like time joined/group size etc
| export default function App() { | ||
| const { | ||
| queue, | ||
| currentUser, | ||
| position, | ||
| peopleAhead, | ||
| addToQueue, | ||
| removeFromQueue | ||
| } = useQueue(); | ||
|
|
||
| return ( | ||
| <div className="min-h-screen bg-[#0e1621] flex items-center justify-center p-4"> | ||
| <div className="w-full max-w-[375px] h-[812px] bg-[#17212b] rounded-3xl shadow-2xl overflow-hidden flex flex-col"> | ||
|
|
||
| {/* Fixed Header */} | ||
| <QueueHeader /> | ||
|
|
||
| {/* Conditional Notification */} | ||
| {currentUser && position > 0 && position <= 3 && ( | ||
| <QueueNotification peopleAhead={peopleAhead} /> | ||
| )} | ||
|
|
||
| {/* Scrollable Body */} | ||
| <QueueBody | ||
| currentUser={currentUser} | ||
| position={position} | ||
| peopleAhead={peopleAhead} | ||
| queue={queue} | ||
| onJoin={addToQueue} | ||
| onLeave={() => removeFromQueue(currentUser?.id)} | ||
| /> | ||
|
|
||
| {/* Fixed Footer */} | ||
| <QueueFooter /> | ||
|
|
||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
avoid writing pages in app.tsx unless you are certain that this app will only have one page. However this app would likely have different pages e.g. user view and admin view. Please set up proper project structure and routing logic.
There was a problem hiding this comment.
Please update the README properly. include project brief and introduction. only include necessary details in dev/contributing section e.g. setup dev environment and notable tech decisions
| <div className="bg-[#1e2936] p-6"> | ||
| <h1 className="text-2xl font-bold text-white text-center"> | ||
| NUSC Queuebot SOSD | ||
| </h1> | ||
| </div> |
There was a problem hiding this comment.
would this eventually become a dynamic component? i.e. display the event title as configured by the event host
No description provided.